Build results-first D3 benchmark explorer#87
Conversation
Swap the marketing landing page for a data-first layout: sentence-case copy, denser standings tables, integrity as footnotes, adapters folded into Run, and a verdict-plus-ladder hero without CTA theater. Co-authored-by: Ned Cutler <nedcut@users.noreply.github.com>
Co-authored-by: Ned Cutler <nedcut@users.noreply.github.com>
📝 WalkthroughWalkthroughThe web app now builds a validated benchmark view, replaces the previous results layout with interactive charts and analysis, adds theme switching, and updates navigation, protocol, quickstart, footer, metadata, styling, and publication-claim tests. ChangesBenchmark results redesign
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant App
participant ResultsExplorer
participant Analysis
participant BenchmarkView
App->>BenchmarkView: build validated benchmark view
App->>ResultsExplorer: provide models and selection state
ResultsExplorer->>App: select model or filter results
App->>Analysis: propagate selected model
Analysis->>BenchmarkView: read scores and mechanic outcomes
Analysis-->>App: render ranking and rejection analysis
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/src/components/ResultsExplorer.tsx (1)
493-501: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive hardcoded counts/claims from the canonical benchmark instead of static copy.
The overview hardcodes "eight ... systems" (Lines 495-496), and the same static assumptions appear in the toolbar copy ("All eight rows overlap", "no observed mean reaches the pick-trader bar" at Lines 621-622) and the forest
<desc>("Every published model has a negative paired lift ... below zero" at Lines 66-69). The readouts already usebenchmark.modelCount/modelsAboveBar, so ifleaderboard.jsonchanges these strings silently drift out of sync — and "no observed mean reaches the bar" can directly contradictmodelsAboveBar > 0. This works against the PR's stated goal that all counts come from one canonical source.At minimum, template the count from
benchmark.modelCountand gate the "above bar" phrasing onbenchmark.modelsAboveBar.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/ResultsExplorer.tsx` around lines 493 - 501, Update the ResultsExplorer copy and forest description to derive claims from the canonical benchmark data: replace hardcoded system counts with benchmark.modelCount, and condition the “above bar” wording on benchmark.modelsAboveBar so it does not claim none when models exceed the threshold. Keep all related overview, toolbar, and SVG <desc> text synchronized with these values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/src/index.css`:
- Around line 230-233: Update the mobile logo sizing rule in the .brand selector
to target the rendered img element instead of svg, ensuring the favicon logo
shrinks to 22px by setting its width and height.
---
Nitpick comments:
In `@web/src/components/ResultsExplorer.tsx`:
- Around line 493-501: Update the ResultsExplorer copy and forest description to
derive claims from the canonical benchmark data: replace hardcoded system counts
with benchmark.modelCount, and condition the “above bar” wording on
benchmark.modelsAboveBar so it does not claim none when models exceed the
threshold. Keep all related overview, toolbar, and SVG <desc> text synchronized
with these values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9a8e5242-82e5-46ad-ab46-56d871a316fb
⛔ Files ignored due to path filters (2)
web/bun.lockis excluded by!**/*.lockweb/public/favicon.svgis excluded by!**/*.svg
📒 Files selected for processing (22)
tests/test_publication_claims.pyweb/index.htmlweb/package.jsonweb/scripts/validate_results_data.tsweb/src/App.tsxweb/src/benchmarkData.tsweb/src/components/Adapters.tsxweb/src/components/Analysis.tsxweb/src/components/Charts.tsxweb/src/components/Footer.tsxweb/src/components/Hero.tsxweb/src/components/HowItWorks.tsxweb/src/components/Integrity.tsxweb/src/components/Ladder.tsxweb/src/components/Leaderboard.tsxweb/src/components/Nav.tsxweb/src/components/Quickstart.tsxweb/src/components/Results.tsxweb/src/components/ResultsExplorer.tsxweb/src/index.cssweb/src/lib.tsweb/src/theme.ts
💤 Files with no reviewable changes (7)
- web/src/components/Results.tsx
- web/src/components/Ladder.tsx
- web/src/components/Leaderboard.tsx
- web/src/components/Integrity.tsx
- web/src/components/Adapters.tsx
- web/src/components/Hero.tsx
- web/src/components/Charts.tsx
| .brand svg { | ||
| width: 24px; | ||
| height: 24px; | ||
| width: 22px; | ||
| height: 22px; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stale selector: .brand svg no longer matches the logo.
Logo now renders an <img src=favicon.svg> instead of inline SVG, so this mobile rule no longer applies and the logo stays at its default 24px on narrow screens instead of shrinking to 22px. Target the <img> (or drive size via the size prop).
🔧 Proposed fix
- .brand svg {
+ .brand img {
width: 22px;
height: 22px;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .brand svg { | |
| width: 24px; | |
| height: 24px; | |
| width: 22px; | |
| height: 22px; | |
| } | |
| .brand img { | |
| width: 22px; | |
| height: 22px; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/src/index.css` around lines 230 - 233, Update the mobile logo sizing rule
in the .brand selector to target the rendered img element instead of svg,
ensuring the favicon logo shrinks to 22px by setting its width and height.
Summary
This is the implementation successor to #83.
Polish pass
The polish direction was independently reviewed by Claude Fable 5 before implementation.
Scientific integrity
src/data/leaderboard.jsonValidation
bun run lintbun run builduv run --extra dev python -m ruff format --check gm_bench examples testsuv run --extra dev python -m ruff check gm_bench examples testsuv run --extra dev python -m pytest -q— 474 passedSummary by CodeRabbit
New Features
Enhancements
Bug Fixes